Skip to content

Two more fixes for non-UTF-8 tests#7681

Merged
aitap merged 2 commits intomasterfrom
native_file_enc
Apr 8, 2026
Merged

Two more fixes for non-UTF-8 tests#7681
aitap merged 2 commits intomasterfrom
native_file_enc

Conversation

@aitap
Copy link
Copy Markdown
Member

@aitap aitap commented Mar 22, 2026

Tests 1966.* failed on my Windows 7 VM where I test data.table with old versions of R. ö cannot be represented in CP1251, and enc2native() converted it to a plain unaccented o. If the characters cannot be represented in the ANSI encoding, we might as well skip the tests. (What if it returns NA or ? on a different system?)

Test 1164.1 shouldn't require the characters to be represented in the native encoding, because it only uses UTF-8 and Latin-1. Both match() and chmatch() offer a strong enough guarantee. Tested on the same Windows 7 VM, and also using LC_ALL=zh_CN.gb2312 luit R CMD check (GB2312 doesn't have ä or ß) and LC_ALL=C on GNU/Linux.

aitap added 2 commits March 22, 2026 16:23
@aitap
Windows path tests: skip if not representable
d2ba0c2 
Not all ANSI encodings can represent accented Latin characters. For
non-representable strings, enc2native() may substitute a different
character (observed: "o" instead of "o with an umlaut), which fails a
later comparison of the file name with the original, non-converted
string.
@aitap
Drop native encoding requirement
6c0c24e 
Test 1164.1 should pass with UTF-8 encoded strings without converting
them to the native encoding.
@aitap aitap requested a review from MichaelChirico as a code owner March 22, 2026 13:43
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 22, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.04%. Comparing base (7db13b9) to head (6c0c24e).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #7681   +/-   ##
=======================================
  Coverage   99.04%   99.04%           
=======================================
  Files          87       87           
  Lines       17031    17031           
=======================================
  Hits        16868    16868           
  Misses        163      163

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tdhock
Copy link
Copy Markdown
Member

tdhock commented Mar 30, 2026

thanks!
do you think it would be worth adding a CI job to test a different locale?
pandas does
image

@aitap
Copy link
Copy Markdown
Member Author

aitap commented Apr 8, 2026

Technically, lin-rel-vanilla currently tests with LC_ALL=C (because the container image doesn't have any locales installed). The language of the messages doesn't matter as much for R CMD check because (unlike interactive test.data.table()) it runs with LANGUAGE=en, but the session encoding does.

Testing with LC_CTYPE = fr_CA.ISO-8859-1, zh_CN.gb2312, ru_RU.KOI8-R might help prevent some future bugs, but not the 1966.* failure because that was a Windows-only problem. (I know there are "locale emulators" for Windows, but I haven't used them with R.)

@aitap aitap merged commit 76ec862 into master Apr 8, 2026
13 checks passed
@aitap aitap deleted the native_file_enc branch April 8, 2026 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tdhock tdhock tdhock approved these changes

@MichaelChirico MichaelChirico Awaiting requested review from MichaelChirico MichaelChirico is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@aitap @tdhock